ci: gate the ~1,830 tests that had no job, and fix the 6 that blocked it - #525
Merged
Conversation
Two dedicated jobs already exist for single files that rotted unnoticed (tests/test_pr_review_watcher.py, tests/integration/reviewer) — each added AFTER that suite had drifted. This generalises the fix instead of waiting for the next one. What the gap cost, concretely: #509 shipped a regression that broke tests/maintenance/, found a week later by hand in #513. #521 merged a brand new failing test in tests/test_dependency_check.py — CI green, reviewer verdict SUCCESS, main red until #522. tests/test_proposer_entrypoint.py had been failing since the board-seam migration weeks earlier and nothing noticed. Switching the gate on required fixing 6 failures, and they were not what they looked like: FOUR WERE BAD MOCKS, NOT BUGS. They raised FileNotFoundError("message") — with no errno. pathlib's predicates swallow only ignorable errnos (ENOENT, ENOTDIR, EBADF, ELOOP), so an errno-less error escapes is_file()/is_dir(), which a real vanished path never does. Verified identically on 3.11 and 3.12: deleting a directory mid-scan makes glob() return []. The mocks were testing CPython's exception plumbing, not the collectors' guards. They now raise the way the OS does. My first attempt "fixed" production by guarding the walk against deletion — defending against something that cannot happen — and is reverted. ONE WAS A REAL GAP THE BAD MOCK WAS HIDING. EACCES and EIO are NOT ignorable, so a log directory that becomes unreadable (rather than deleted) does raise straight out of the walk and takes the collector with it. latest_matching_file now guards the walk for OSError, with the distinction written down, because "return None when nothing is discoverable" is its contract. ONE WAS A REAL PRODUCTION BUG. custodian_sweep._emit answered zero-findings before its dry-run branch could run, so that branch was unreachable and a dry run reported "skipped-zero-findings" — the past tense, for work it had not done. Two tests in the same file asserted opposite labels for the same call; the module's own would-comment / would-create convention and the shadowed branch both say `would-` was intended. ONE WAS A STALE PATCH TARGET: proposer.main.PlaneClient stopped existing at the board migration. Patched at the seam now. Verified: tests/unit 8623 passed, the new job's exact command 1830 passed, both 0 failed. ruff clean, ty at the 13-diagnostic baseline, custodian doctor OK, audit clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new test-rest job went red on its first CI run — which is the job doing its
job. Three failures, all in dependency_drift, none reproducible on 3.12:
CPython 3.11's glob() stats every matched path through exists(); 3.12's does
not. Two of the tests counted Path.stat calls to prove the collector does not
re-stat after discovery — a fair question, but the counter was also counting
the interpreter's probing. The third needed one unreadable run directory to be
skipped while the rest are still read.
My first attempt made it worse: wrapping the walk in list(glob(...)) meant a
single bad entry aborted discovery, turning "skip run1, use run2" into
not_available. glob() is a generator, so the first error closes it — per-entry
recovery inside it is not possible at all.
_latest_dependency_report now walks with iterdir(), which stats nothing:
* one unreadable entry is skipped, not fatal (EACCES/EIO are NOT among the
errnos pathlib swallows, unlike ENOENT/ENOTDIR/EBADF/ELOOP)
* the counting assertions see only the collector's own stat calls, on either
interpreter, so the test-side glob patching I added is removed as both
unnecessary and ineffective — it still called the real glob()
Verified on both: 3.12 locally 1830 passed, 3.11 in a python:3.11-slim
container 1828 passed with the 3 fixed. The container also reports 2 failures
CI does not (test_resolve_repos_root_falls_back_to_checkout_layout,
test_loader_reads_latest_snapshot_with_bounded_history) — it mounts a flat
worktree with no sibling checkouts, which is what those two probe. CI is the
authority for CI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 21, 2026
…le backlog items half that is genuinely cosmetic and leaves the half that is not. Renames (each had exactly one importer or none, no behavior change): * `propagation/plane_adapter.py` -> `board_adapter.py`, `PlaneTaskCreator` -> `BoardTaskCreator`. Docstrings in `propagator.py` still pointed at `PlaneClient.create_issue`, a class deleted in #521. * `tests/test_plane_parsing.py` -> `test_task_parsing.py` — it tests `TaskParser` and never touched Plane. * `docs/design/plane_kodo_wrapper.md` -> `docs/history/plane-kodo-wrapper.md`, `status: implemented` -> `superseded`. It sat among live design docs describing a retired board AND a retired engine. And one that was not cosmetic. `config/plane_task_template.example.md` was a stale copy of `render_task_template()` output (it still said "the change you want Kodo to make"). No code read it — `oc setup` writes `config/task_template.local.md`. But `.gitignore`, `docs/operator/setup.md` and BOTH secrets scripts still named the dead `plane_task_template.local.md` path, so `backup-secrets.sh` was backing up a file that cannot exist and the operator's real template was in no backup at all. All four repointed; the stale example deleted. Also dropped `.gitignore` entries for `deployment/plane/**` and `tools/report/kodo_plane/` — no such directories. Left alone on purpose: `plane_task_id` (read from on-disk review state), `plane_issue_id` (read from proposer artifacts), and `"plane"` as an alert channel name (validated against operator config). Those are wire formats; a bare rename orphans in-flight reviews. Filed in Up Next as a write-both migration. Both Up Next items were already done and just unrecorded — moved to Done with evidence: the ~1,830-test CI gap is covered by the `test-rest` job (`pytest tests/ --ignore=tests/unit`, #525, survived the #527 port), and `audit` on Forgejo Actions is live (runner registered, workflow ported to `.forgejo/workflows/custodian-audit.yml`, branch protection requiring `custodian-audit / audit (pull_request)` with apply_to_admins). Verified with imports forced to the worktree (PYTHONPATH), not the live checkout the fleet runs from: 67 targeted tests pass, ruff check and format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 21, 2026
…le backlog items half that is genuinely cosmetic and leaves the half that is not. Renames (each had exactly one importer or none, no behavior change): * `propagation/plane_adapter.py` -> `board_adapter.py`, `PlaneTaskCreator` -> `BoardTaskCreator`. Docstrings in `propagator.py` still pointed at `PlaneClient.create_issue`, a class deleted in #521. * `tests/test_plane_parsing.py` -> `test_task_parsing.py` — it tests `TaskParser` and never touched Plane. * `docs/design/plane_kodo_wrapper.md` -> `docs/history/plane-kodo-wrapper.md`, `status: implemented` -> `superseded`. It sat among live design docs describing a retired board AND a retired engine. And one that was not cosmetic. `config/plane_task_template.example.md` was a stale copy of `render_task_template()` output (it still said "the change you want Kodo to make"). No code read it — `oc setup` writes `config/task_template.local.md`. But `.gitignore`, `docs/operator/setup.md` and BOTH secrets scripts still named the dead `plane_task_template.local.md` path, so `backup-secrets.sh` was backing up a file that cannot exist and the operator's real template was in no backup at all. All four repointed; the stale example deleted. Also dropped `.gitignore` entries for `deployment/plane/**` and `tools/report/kodo_plane/` — no such directories. Left alone on purpose: `plane_task_id` (read from on-disk review state), `plane_issue_id` (read from proposer artifacts), and `"plane"` as an alert channel name (validated against operator config). Those are wire formats; a bare rename orphans in-flight reviews. Filed in Up Next as a write-both migration. Both Up Next items were already done and just unrecorded — moved to Done with evidence: the ~1,830-test CI gap is covered by the `test-rest` job (`pytest tests/ --ignore=tests/unit`, #525, survived the #527 port), and `audit` on Forgejo Actions is live (runner registered, workflow ported to `.forgejo/workflows/custodian-audit.yml`, branch protection requiring `custodian-audit / audit (pull_request)` with apply_to_admins). Verified with imports forced to the worktree (PYTHONPATH), not the live checkout the fleet runs from: 67 targeted tests pass, ruff check and format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Aug 21, 2026
…le backlog items (#529) half that is genuinely cosmetic and leaves the half that is not. Renames (each had exactly one importer or none, no behavior change): * `propagation/plane_adapter.py` -> `board_adapter.py`, `PlaneTaskCreator` -> `BoardTaskCreator`. Docstrings in `propagator.py` still pointed at `PlaneClient.create_issue`, a class deleted in #521. * `tests/test_plane_parsing.py` -> `test_task_parsing.py` — it tests `TaskParser` and never touched Plane. * `docs/design/plane_kodo_wrapper.md` -> `docs/history/plane-kodo-wrapper.md`, `status: implemented` -> `superseded`. It sat among live design docs describing a retired board AND a retired engine. And one that was not cosmetic. `config/plane_task_template.example.md` was a stale copy of `render_task_template()` output (it still said "the change you want Kodo to make"). No code read it — `oc setup` writes `config/task_template.local.md`. But `.gitignore`, `docs/operator/setup.md` and BOTH secrets scripts still named the dead `plane_task_template.local.md` path, so `backup-secrets.sh` was backing up a file that cannot exist and the operator's real template was in no backup at all. All four repointed; the stale example deleted. Also dropped `.gitignore` entries for `deployment/plane/**` and `tools/report/kodo_plane/` — no such directories. Left alone on purpose: `plane_task_id` (read from on-disk review state), `plane_issue_id` (read from proposer artifacts), and `"plane"` as an alert channel name (validated against operator config). Those are wire formats; a bare rename orphans in-flight reviews. Filed in Up Next as a write-both migration. Both Up Next items were already done and just unrecorded — moved to Done with evidence: the ~1,830-test CI gap is covered by the `test-rest` job (`pytest tests/ --ignore=tests/unit`, #525, survived the #527 port), and `audit` on Forgejo Actions is live (runner registered, workflow ported to `.forgejo/workflows/custodian-audit.yml`, branch protection requiring `custodian-audit / audit (pull_request)` with apply_to_admins). Verified with imports forced to the worktree (PYTHONPATH), not the live checkout the fleet runs from: 67 targeted tests pass, ruff check and format clean. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
~1,830 tests had no CI job at all — everything under
tests/maintenance/,tests/observer/,tests/verdicts/, the rest oftests/integration/, and the top-leveltests/test_*.py.Two dedicated jobs already exist for single files that rotted unnoticed (
tests/test_pr_review_watcher.py,tests/integration/reviewer) — each added after that suite had drifted. This generalises the fix instead of waiting for the next one.What the gap cost, concretely
tests/maintenance/— found a week later by hand in fix(adapters): the board factory rejected a settings double #513.tests/test_dependency_check.py. CI green, reviewer verdict SUCCESS, main red until test(dependency-check): fix the assertion that left main red #522.tests/test_proposer_entrypoint.pyhad been failing since the board-seam migration weeks earlier. Nothing noticed.The 6 blockers were not what they looked like
Four were bad mocks, not bugs. They raised
FileNotFoundError("message")— with no errno. pathlib's predicates swallow only ignorable errnos (ENOENT, ENOTDIR, EBADF, ELOOP), so an errno-less error escapesis_file()/is_dir(), which a real vanished path never does. Verified identically on 3.11 and 3.12: deleting a directory mid-scan makesglob()return[]. The mocks were testing CPython's exception plumbing, not the collectors' guards.My first attempt "fixed" production by guarding the walk against deletion — defending against something that cannot happen — and is reverted in this branch.
One was a real gap the bad mock was hiding.
EACCESandEIOare not ignorable, so a log directory that becomes unreadable (rather than deleted) does raise straight out of the walk and take the collector with it.latest_matching_filenow guards forOSError, with the distinction written down, because "return None when nothing is discoverable" is its contract.One was a real production bug.
custodian_sweep._emitanswered zero-findings before its dry-run branch could run, so that branch was unreachable and a dry run reportedskipped-zero-findings— the past tense, for work it had not done. Two tests in the same file asserted opposite labels for the same call; the module's ownwould-comment/would-createconvention and the shadowed branch both saywould-was intended.One was a stale patch target —
proposer.main.PlaneClientstopped existing at the board migration.Verification
tests/unitThe new job is not added to branch protection's required contexts, so it changes no merge policy directly — but the reviewer's merge gate refuses on any failed check, so a regression here blocks the fleet the same way.
🤖 Generated with Claude Code